Skip to content

Remove path filter so that required checks can be used#7

Merged
jackgerrits merged 1 commit intomainfrom
jackgerrits-patch-1
May 9, 2025
Merged

Remove path filter so that required checks can be used#7
jackgerrits merged 1 commit intomainfrom
jackgerrits-patch-1

Conversation

@jackgerrits
Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings May 9, 2025 14:56
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the path filter from the GitHub Actions Python check workflow to ensure that required checks run on all commits regardless of the files changed.

  • Removal of specific path filters in the workflow file.
  • Ensures that the check runs on any commit in the pull request.

@jackgerrits jackgerrits enabled auto-merge May 9, 2025 15:00
@jackgerrits jackgerrits disabled auto-merge May 9, 2025 15:02
@jackgerrits jackgerrits merged commit 3257113 into main May 9, 2025
2 checks passed
@crickman crickman deleted the jackgerrits-patch-1 branch July 22, 2025 16:43
ReubenBond pushed a commit to ReubenBond/agent-framework that referenced this pull request Oct 28, 2025
github-merge-queue Bot pushed a commit that referenced this pull request Apr 10, 2026
* Harden Python checkpoint persistence defaults

Add RestrictedUnpickler to _checkpoint_encoding.py that limits which
types may be instantiated during pickle deserialization.  By default
FileCheckpointStorage now uses the restricted unpickler, allowing only:

- Built-in Python value types (primitives, datetime, uuid, decimal,
  collections, etc.)
- All agent_framework.* internal types
- Additional types specified via the new allowed_checkpoint_types
  parameter on FileCheckpointStorage

This narrows the default type surface area for persisted checkpoints
while keeping framework-owned scenarios working without extra
configuration.  Developers can extend the allowed set by passing
"module:qualname" strings to allowed_checkpoint_types.

The decode_checkpoint_value function retains backward-compatible
unrestricted behavior when called without the new allowed_types kwarg.

Fixes #4894

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: resolve mypy no-any-return error in checkpoint encoding

Add explicit type annotation for super().find_class() return value
to satisfy mypy's no-any-return check.

Fixes #4894

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Simplify find_class return in _RestrictedUnpickler (#4894)

Remove unnecessary intermediate variable and apply # noqa: S301 # nosec
directly on the super().find_class() call, matching the established
pattern used on the pickle.loads() call in the same file.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address review feedback for #4894: Python: Harden Python checkpoint persistence defaults

* Restore # noqa: S301 on line 102 of _checkpoint_encoding.py (#4894)

The review feedback correctly identified that removing the # noqa: S301
suppression from the find_class return statement would cause a ruff S301
lint failure, since the project enables bandit ("S") rules. This
restores consistency with lines 82 and 246 in the same file.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address review feedback for #4894: Python: Harden Python checkpoint persistence defaults

* Address PR review comments on checkpoint encoding (#4894)

- Move module docstring to proper position after __future__ import
- Fix find_class return type annotation to type[Any]
- Add missing # noqa: S301 pragma on find_class return
- Improve error message to reference both allowed_types param and
  FileCheckpointStorage.allowed_checkpoint_types
- Add -> None return annotation to FileCheckpointStorage.__init__
- Replace tempfile.mktemp with TemporaryDirectory in test
- Replace contextlib.suppress with pytest.raises for precise assertion
- Remove unused contextlib import

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address PR #4941 review comments: fix docstring position and return type

- Move module docstring before 'from __future__' import so it populates
  __doc__ (comment #4)
- Change find_class return annotation from type[Any] to type to avoid
  misleading callers about non-type returns like copyreg._reconstructor
  (comment #2)

Comments #1, #3, #5, #6, #7, #8 were already addressed in the current code.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address review feedback for #4894: review comment fixes

* fix: use pickle.UnpicklingError in RestrictedUnpickler and improve docstring (#4894)

- Change _RestrictedUnpickler.find_class to raise pickle.UnpicklingError
  instead of WorkflowCheckpointException, since it is pickle-level concern
  that gets wrapped by the caller in _base64_to_unpickle.
- Remove now-unnecessary WorkflowCheckpointException re-raise in
  _base64_to_unpickle (pickle.UnpicklingError is caught by the generic
  except Exception handler and wrapped).
- Expand decode_checkpoint_value docstring to show a concrete example of
  the module:qualname format with a user-defined class.
- Add regression test verifying find_class raises pickle.UnpicklingError.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: address PR #4941 review comments for checkpoint encoding

- Comment 1 (line 103): Already resolved in prior commit — _RestrictedUnpickler
  now raises pickle.UnpicklingError instead of WorkflowCheckpointException.

- Comment 2 (line 140): Add concrete usage examples to decode_checkpoint_value
  docstring showing both direct allowed_types usage and FileCheckpointStorage
  allowed_checkpoint_types usage. Rename 'SafeState' to 'MyState' across all
  docstrings for consistency, making it clear this is a user-defined class name.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: replace deprecated 'builtin' repo with pre-commit-hooks in pre-commit config

pre-commit 4.x no longer supports 'repo: builtin'. Merge those hooks into
the existing pre-commit-hooks repo entry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* style: apply pyupgrade formatting to docstring example

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: resolve pre-commit hook paths for monorepo git root

The poe-check and bandit hooks referenced paths relative to python/
but pre-commit runs hooks from the git root (monorepo root). Fix
poe-check entry to cd into python/ first, and update bandit config
path to python/pyproject.toml.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix pre-commit config paths for prek --cd python execution

Revert bandit config path from 'python/pyproject.toml' to 'pyproject.toml'
and poe-check entry from explicit 'cd python' wrapper to direct invocation,
since prek --cd python already sets the working directory to python/.

Also apply ruff formatting fixes to cosmos checkpoint storage files.

Fixes #4894

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: add builtins:getattr to checkpoint deserialization allowlist

Pickle uses builtins:getattr to reconstruct enum members (e.g.,
WorkflowMessage.type which is a MessageType enum). Without it in the
allowlist, checkpoint roundtrip tests fail with
WorkflowCheckpointException.

Fixes #4894

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address review feedback for #4894: review comment fixes

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
eavanvalkenburg added a commit to eavanvalkenburg/agent-framework that referenced this pull request Apr 24, 2026
ADR 0026:
- Tighten Decision Outcome Summary so each concept is mentioned once;
  defer full definitions to the Terminology section.
- Update ChannelInvocationHook bullet to match the clarified gap microsoft#7
  language (uniform ChannelRequest envelope, hook timing, illustrative
  examples).
- Drop Decision Drivers bullets that just restated Business Goals;
  cross-link to the goals section instead.
- Replace the More Information bullet list with a pointer to Non-Goals.

Spec 002:
- Trim requirement microsoft#21 to point at the canonical LinkPolicy section
  instead of restating the full contract.
- Add a #linkpolicy-and-trust_level subsection anchor for cross-refs.
- Trim the Terminology LinkPolicy entry's two-hosts caveat (canonical
  version stays in the Key Types section).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
eavanvalkenburg added a commit to eavanvalkenburg/agent-framework that referenced this pull request Apr 26, 2026
ADR 0026:
- Tighten Decision Outcome Summary so each concept is mentioned once;
  defer full definitions to the Terminology section.
- Update ChannelInvocationHook bullet to match the clarified gap microsoft#7
  language (uniform ChannelRequest envelope, hook timing, illustrative
  examples).
- Drop Decision Drivers bullets that just restated Business Goals;
  cross-link to the goals section instead.
- Replace the More Information bullet list with a pointer to Non-Goals.

Spec 002:
- Trim requirement microsoft#21 to point at the canonical LinkPolicy section
  instead of restating the full contract.
- Add a #linkpolicy-and-trust_level subsection anchor for cross-refs.
- Trim the Terminology LinkPolicy entry's two-hosts caveat (canonical
  version stays in the Key Types section).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
eavanvalkenburg added a commit that referenced this pull request May 5, 2026
* first iteration of channel spec

* added deny link setup

* clarify invocation hook role and dedupe ADR/spec

ADR 0026:
- Tighten Decision Outcome Summary so each concept is mentioned once;
  defer full definitions to the Terminology section.
- Update ChannelInvocationHook bullet to match the clarified gap #7
  language (uniform ChannelRequest envelope, hook timing, illustrative
  examples).
- Drop Decision Drivers bullets that just restated Business Goals;
  cross-link to the goals section instead.
- Replace the More Information bullet list with a pointer to Non-Goals.

Spec 002:
- Trim requirement #21 to point at the canonical LinkPolicy section
  instead of restating the full contract.
- Add a #linkpolicy-and-trust_level subsection anchor for cross-refs.
- Trim the Terminology LinkPolicy entry's two-hosts caveat (canonical
  version stays in the Key Types section).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* updated adr and spec

* Update hosting channels ADR and spec

- Document FoundryHostedAgentHistoryProvider roundtrip of additional_properties namespaces via the agent_framework container key on stored OutputItems.
- Add Foundry storage gap subsection capturing the update_item service ask required for post-push delivery_tracking[] mutation.
- Triage open questions: 18 resolved (now in a Resolved Questions decisions log), 3 notes-updated, 6 unchanged. Capture spec-body follow-ups implied by the resolutions in a new Decisions-driven follow-ups subsection.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Refine hosting ADR + spec: A2A/MCP-tool channels, store-parameter matrix, open-question pass

- Surface A2A and MCP-tool channels as explicitly designed-in but fast-follow work after the first Responses + Invocations + Telegram release. Updated ADR business goals, non-goals, and More Information; added spec reqs #25 (A2AChannel) and #26 (MCPToolChannel) under v1 Fast Follow; renumbered the WhatsApp/Teams entry to #27.
- New 'The Responses store parameter' subsection in the spec: 2x3 destination matrix making explicit that 'store' has no canonical meaning at the hosted-agent layer — the developer decides what it maps to across service-side, hosted-agent storage, and caller-side. Includes design properties on forwarding-vs-mapping, per-deployment documentation responsibility, and richer storage vocabulary via OpenAI's extra_body.
- Fixed contradicting spec text that previously claimed ResponsesChannel maps store=False to session_mode=disabled by default; updated channel options table, session_mode terminology entry, and Scenario 3 prose/comment to match the new model.
- Renamed FoundryHistoryProvider -> FoundryHostedAgentHistoryProvider throughout the spec (9 occurrences) so the name reinforces the intended hosted-agent use case.
- ADR open-questions pass: walked through all 15 entries with the user. 13 resolved (moved to a new 'Resolved Questions (decisions log)' table), 2 kept open with refined wording (Q6 'Channel' GA name, Q14 Responses WS subprotocol). Added a 'Decisions-driven follow-ups' bullet list capturing the spec-body / sample edits implied by the resolutions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Hosting ADR + spec: rename Teams channel to Activity Protocol, add multi-user conversation design

- Rename the planned Teams channel to ActivityChannel (package agent-framework-hosting-activity). Promoted to req #27 (v1 fast follow) alongside A2A and MCP-tool, with native translations from Activity Protocol objects to AF types so the contract is explicit rather than implicit through Invocations. Channel sits behind Azure Bot Service, which fronts Teams / Web Chat / Slack / etc. Naming reserves a TeamsChannel name for any future direct-to-Teams transport that bypasses Bot Service (now stretch req #28 with WhatsApp). ResponseTarget channel ids and JSON examples updated from "teams" to "activity". Appendix B updated to acknowledge that ActivityChannel deliberately reuses the Bot Service connector model (the no-connector stance applies to the rest of the channel set).

- Add first-class design for multi-user surfaces (Telegram groups / supergroups / forum topics; Activity Protocol groupChat and team channels). Cleanly separate user identity (ChannelIdentity.native_id = from.id / from.aadObjectId) from conversation locator (ChannelRequest.conversation_id = chat.id (+ message_thread_id / replyToId)). New per-channel options: conversation_scope (per_user / per_user_per_conversation (default in groups) / per_conversation) and accept_in_group addressing rule (mention_only (default) / command_only / mention_or_command / all). Specifies originating reply must include conversation + thread locator, ChannelPush behavior in groups, link-ceremony privacy (challenges redirected to user DMs), and the Activity-channel mapping for personal / groupChat / channel conversationType plus Teams replyToId threading. Broadcast Telegram Channels and adaptive-card Invoke activity flows scoped as fast follow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs(hosting): rename RunHandle → ContinuationToken; HostStateStore (file-based v1); align agentserver dependency posture

- Rename RunHandle → ContinuationToken (opaque URL-safe `token` field) throughout
  ADR + spec; update routes to /{continuation_token}; spec out equivalent
  continuation-token support for the Invocations channel (Q20 done).
- Introduce HostStateStore as the single persistence seam for host-execution
  metadata (continuation tokens, identity-link grants, last-seen records).
  V1 default: FileHostStateStore (atomic JSON-per-record under ./.af-hosting/,
  per-namespace TTLs) — background runs and link grants now survive host
  restarts. InMemoryHostStateStore for tests; pluggable Cosmos / SQL / Redis
  remain v1 fast follow under req #23. Closes Q9, Q11, Q14.
- Drop blanket "no agentserver dependency" claims. Hosting core is still
  independent of agentserver, but channel packages MAY consume lower-level
  building blocks (notably the Foundry response-store SDK that
  FoundryHostedAgentHistoryProvider builds on).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs(hosting): swap Scenarios 6 and 7 so the linker comes before cross-channel continuity

Scenario 6 (cross-channel continuity) previously forward-referenced Scenario 7
(linker) twice, since continuity depends on the link/merge ceremony. Invert the
order so the linker scenario establishes the mechanism first and the continuity
scenario builds on it. Update internal cross-references, the require_link
section anchor, and Scenario 8's prerequisites/comment to match. Also tightened
the new Scenario 7's closing note to point at HostStateStore (file-based
default) for cross-host continuity, and dropped a stale MfaIdentityLinker
reference from the linker variants paragraph (Q13 dropped MFA from phase 1).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs(hosting): rewrite Scenario 7 as trusted-relay + add ResponseTarget.identities

The previous Scenario 7 (cross-channel chat continuity) implied two independent
auto-issued isolation_keys would converge by themselves — they don't, that
needs a linker. Replace with a more realistic and complementary scenario:
a trusted server-side application backend exposes Responses + Telegram against
the same agent and uses extra_body to carry app-internal identity hints
(app_user_id, push_to_telegram_chat_id) that a Responses run_hook translates
into both an isolation_key promotion and a push to a known Telegram chat.
Includes a closing variant pointing back at Scenario 6's linker for the
no-app-table flow.

Adds the ResponseTarget.identities([ChannelIdentity(...)]) variant to the
type table and req #12 to support 'caller already knows the channel-native
recipient' delivery without going through the link store. Bypasses the link
store but still consults LinkPolicy per delivery.

Drops MfaIdentityLinker references from req #11, req #24, and the linker
helpers table (Q13 had already dropped MFA from phase 1; the spec body just
hadn't caught up). Marks ADR Q8 follow-up done.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs(hosting): wire FileCheckpointStorage into Scenario 9 + show resume-from-checkpoint flow

Scenario 9 now builds the workflow with a FileCheckpointStorage so executor
frames are persisted across runs, and demonstrates how the run_hook surfaces
a caller-supplied resume_from_checkpoint into request.attributes so the host's
workflow dispatch can pass it to Workflow.run(checkpoint_id=...). Closing
paragraph clarifies that CheckpointStorage is workflow-runtime state, kept
structurally separate from HostStateStore and ContextProvider — three
protocols that MAY share a backend but stay independently typed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs(hosting): emphasize result richness in Scenario 10 (channels are not limited to result.text)

Add a 'Result is rich, not just text' callout under the channel-authoring
sample. Inventories the typed Contents on the underlying AgentRunResult
(TextContent, DataContent, UriContent, FunctionCallContent /
FunctionResultContent, HostedFile/VectorStoreContent, UsageContent,
TextReasoningContent, ErrorContent + additional_properties), the typed
structured output via result.value, and shows concrete examples per channel
shape: Telegram (MarkdownV2 + sendPhoto/sendAudio + inline keyboards),
Responses (full content-list round-trip), chat UI (GFM/HTML +
collapsible tool/reasoning panels), voice (TTS + earcons), typed RPC
(result.value first). result.text is positioned as a convenience for
single-string channels, not the contract.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* spec: add TeamsChannel (microsoft/teams.py) as fast-follow req #28

Add a Teams-native channel package built on the MIT-licensed
microsoft/teams.py SDK as fast-follow alongside the generic
ActivityChannel (req #27). Where ActivityChannel targets the
generic Activity Protocol surface, TeamsChannel exploits
Teams-specific affordances the generic protocol does not surface
natively: Adaptive Cards (typed builder), streamed replies,
AI-generated badge, feedback controls + form, suggested-prompt
chips, inline citations, modal Dialogs, Message Extensions
(action / search / link unfurling), proactive / targeted /
threaded messages, and SSO via MSAL.

Mounts the SDK's App into the host's Starlette app via a custom
HttpServerAdapter; reuses the same host-tracked-session family
as ActivityChannel (from.aadObjectId -> ChannelIdentity). The
SDK already ships a 'Build an agent using Microsoft Agent
Framework' guide so the integration story is direct.

Renumber the WhatsApp / direct-to-Teams stretch item to req #29
and clarify its 'direct-to-Teams' placeholder is a future
transport that bypasses both Bot Service and the teams.py SDK.

Add the SDK to Dependencies & Commitment Status as a proposed
runtime dep of agent-framework-hosting-teams.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* spec: clarify direct-to-Teams stretch as speculative (no Bot Service)

Split the WhatsApp + direct-to-Teams stretch entry into two
distinct items and reword the direct-to-Teams item to be honest
about its current feasibility:

- It MUST not rely on Azure Bot Service (otherwise it is just
  ActivityChannel / TeamsChannel under a different name).
- No such transport is publicly available today: Graph chat APIs
  and microsoft/teams.py both ultimately route through Bot Service
  for the bot-as-conversation-participant pattern.
- The slot is kept on the roadmap to preserve the naming line in
  case Microsoft ships a Bot-Service-free transport (native Teams
  REST/RPC, a Graph subscription strong enough to drive both
  inbound and outbound message flow, ...).
- Reaffirm TeamsChannel (req #28) as the canonical Teams channel
  until then.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* spec: clarify TeamsChannel still rides on Bot Service in v1; add audience table

Make explicit that TeamsChannel (req #28) uses Azure Bot Service
in v1 — the microsoft/teams.py SDK is a higher-level Pythonic
wrapper over the same Activity Protocol pipeline that
ActivityChannel exposes raw. The difference is what the developer
writes against, not the network path. A Bot-Service-free Teams
transport is not currently possible and stays tracked as the
speculative req #30.

Add the ActivityChannel vs TeamsChannel audience comparison table
to req #28 so the choice is obvious to readers:
- ActivityChannel: maximum portability across all Bot Service-fronted channels.
- TeamsChannel: Teams-first deployments wanting Cards / Dialogs /
  Message Extensions / citations / feedback / suggested prompts /
  SSO out of the box.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants